Skip to content

Drop cname module and create resource directly #7

Closed
wants to merge 1 commit into from

Conversation

mcgin314
Copy link
Contributor

Proposed approach that works for plan.

Comment on lines +108 to +114
resource "aws_route53_record" "entry" {
zone_id = aws_route53_zone.cluster_domain.zone_id
name = "*.${local.cluster_domain_name}"
type = upper(local.record_type)
ttl = local.ttl
records = [var.istio_ingress_lb]
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd really prefer to use the upstream module.

Copy link
Contributor

@morga471 morga471 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer not to do this, need to make sure the plan works though =/

@morga471
Copy link
Contributor

Okay. So I think we have all been wrong.
The thing that has been bugging me is that we aren't adding all of the endpoints for the ALB. The module, this method, the existing method in existing dns module, all use the same "structure" for the same "end_state", that is a CNAME pointing to the ALB.
However, this is AWS. For an ALB in AWS, Route53 has special functionality, an ALIAS record.

  1. This is NOT the same thing as a CNAME.
  2. The special functionality is for the specific use case of an ALB/ELB needing a single endpoint.
  3. there is no ALIAS function in the upstream terraform-modules/aws-dns, which leads me to believe we haven't used this.
resource "aws_route53_record" "www" {
  zone_id = aws_route53_zone.primary.zone_id
  name    = "example.com"
  type    = "A"

  alias {
    name                   = aws_elb.main.dns_name
    zone_id                = aws_elb.main.zone_id
    evaluate_target_health = true
  }
}

The difference, this allows a single record to point to all the records of the ALB.
I am looking to PR the functionality into aws-dns, but we can use this module to figure it out in the meantime. Of course, I'm writing this before testing it, so I could be wrong....

@morga471
Copy link
Contributor

I have implemented the described approach on the lb_cname branch and will open PR#8.

@morga471 morga471 mentioned this pull request Oct 17, 2024
@mcgin314
Copy link
Contributor Author

Closing in favor of using the alias record.

@mcgin314 mcgin314 closed this Oct 17, 2024
@morga471 morga471 deleted the feature-cname-resource branch April 18, 2025 05:10
Sign in to join this conversation on GitHub.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants